Skip to content

fix: identical endpoint name conflicts #1521

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

fix: identical endpoint name conflicts #1521

wants to merge 13 commits into from

Conversation

@akurinnoy
Copy link
Collaborator

@akurinnoy akurinnoy commented Oct 8, 2025 •
edited
Loading

What does this PR do?

This PR implements conflict detection and prevention at both the controller and webhook levels.

What issues does this PR fix or reference?

fixes eclipse-che/che#23231

Is it tested? How?

Same-namespace conflict detection

  1. Create test namespace
oc create namespace test-namespace
  1. Create first namespace and wait for it to be running.
cat << EOF | oc apply -f -
apiVersion: workspace.devfile.io/v1alpha2
kind: DevWorkspace
metadata:
  name: endpoint-test-1
  namespace: test-namespace
spec:
  started: true
  routingClass: basic
  template:
    components:
      - name: postgresql
        container:
          image: quay.io/fedora/postgresql-12:latest
          endpoints:
            - name: postgresql
              targetPort: 5432
              exposure: internal
              protocol: tcp
              attributes:
                discoverable: true
          env:
            - name: POSTGRESQL_USER
              value: user
            - name: POSTGRESQL_PASSWORD
              value: pass
            - name: POSTGRESQL_ROOT_PASSWORD
              value: root
            - name: POSTGRESQL_DATABASE
              value: db
EOF
oc wait --for=condition=Ready dw/endpoint-test-1 -n test-namespace --timeout=300s
  1. Try to create second workspace with same discoverable endpoint name. This operation should fail.
cat << EOF | oc apply -f -
apiVersion: workspace.devfile.io/v1alpha2
kind: DevWorkspace
metadata:
  name: endpoint-test-2
  namespace: test-namespace
spec:
  started: true
  routingClass: basic
  template:
    components:
      - name: postgresql
        container:
          image: quay.io/fedora/postgresql-12:latest
          endpoints:
            - name: postgresql
              targetPort: 5432
              exposure: internal
              protocol: tcp
              attributes:
                discoverable: true
          env:
            - name: POSTGRESQL_USER
              value: user
            - name: POSTGRESQL_PASSWORD
              value: pass
            - name: POSTGRESQL_ROOT_PASSWORD
              value: root
            - name: POSTGRESQL_DATABASE
              value: db
EOF

Expected result:

# "admission webhook "validate-devfile.devworkspace-controller.svc" denied the request: 
# discoverable endpoint 'postgresql' is already in use by workspace 'endpoint-test-1'"

Cross-name isolation

  1. Create second namespace
oc create namespace test-namespace-2
  1. Create workspace in second namespace with same endpoint name
cat << EOF | oc apply -f -
apiVersion: workspace.devfile.io/v1alpha2
kind: DevWorkspace
metadata:
  name: endpoint-test-1
  namespace: test-namespace-2
spec:
  started: true
  routingClass: basic
  template:
    components:
      - name: postgresql
        container:
          image: quay.io/fedora/postgresql-12:latest
          endpoints:
            - name: postgresql
              targetPort: 5432
              exposure: internal
              protocol: tcp
              attributes:
                discoverable: true
          env:
            - name: POSTGRESQL_USER
              value: user
            - name: POSTGRESQL_PASSWORD
              value: pass
            - name: POSTGRESQL_ROOT_PASSWORD
              value: root
            - name: POSTGRESQL_DATABASE
              value: db
EOF
  1. Verify both workspaces are running successfully
oc wait --for=condition=Ready dw/endpoint-test-1 -n test-namespace --timeout=300s
oc wait --for=condition=Ready dw/endpoint-test-1 -n test-namespace-2 --timeout=300s
  1. Verify both workspaces have services with the same name
oc get svc postgresql -n test-namespace
oc get svc postgresql -n test-namespace-2

PR Checklist

  • E2E tests pass (when PR is ready, comment /test v8-devworkspace-operator-e2e, v8-che-happy-path to trigger)
    • v8-devworkspace-operator-e2e: DevWorkspace e2e test
    • v8-che-happy-path: Happy path for verification integration with Che

@akurinnoy akurinnoy self-assigned this Oct 8, 2025
tolusha
tolusha reviewed Oct 8, 2025
View reviewed changes
@akurinnoy akurinnoy marked this pull request as draft October 8, 2025 14:26
tolusha
tolusha reviewed Oct 9, 2025
View reviewed changes
@akurinnoy
chore: remove excessive logs.
4879878 
Signed-off-by: Oleksii Kurinnyi <[email protected]>
@akurinnoy
chore: consistent error
dc0a395 
Signed-off-by: Oleksii Kurinnyi <[email protected]>
@akurinnoy
fixup! chore: consistent error
c7ddd74 
Signed-off-by: Oleksii Kurinnyi <[email protected]>
@akurinnoy
fixup! fixup! chore: consistent error
91f1014 
Signed-off-by: Oleksii Kurinnyi <[email protected]>
@akurinnoy
fixup! fixup! fixup! chore: consistent error
9de6b51 
Signed-off-by: Oleksii Kurinnyi <[email protected]>
@akurinnoy
chore: check terminating workspaces
338237f 
Signed-off-by: Oleksii Kurinnyi <[email protected]>
@openshift-ci openshift-ci bot added the lgtm label Oct 9, 2025
@openshift-ci
Copy link

openshift-ci bot commented Oct 9, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: akurinnoy, tolusha
Once this PR has been reviewed and has the lgtm label, please assign dkwon17 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@akurinnoy akurinnoy marked this pull request as ready for review October 9, 2025 10:16
@openshift-ci openshift-ci bot removed the lgtm label Oct 9, 2025
@openshift-ci
Copy link

openshift-ci bot commented Oct 9, 2025

New changes are detected. LGTM label has been removed.

rohanKanojia
rohanKanojia reviewed Oct 17, 2025
View reviewed changes
rohanKanojia
rohanKanojia reviewed Oct 17, 2025
View reviewed changes
rohanKanojia
rohanKanojia reviewed Oct 17, 2025
View reviewed changes
@rohanKanojia
Copy link
Member

rohanKanojia commented Oct 17, 2025

I tested the PR with abovementioned steps and it seems to work as expected ✔️

@akurinnoy
test: fix feedback
44d04a8 
Signed-off-by: Oleksii Kurinnyi <[email protected]>
@akurinnoy
fixup! test: fix feedback
1ea13cf 
Signed-off-by: Oleksii Kurinnyi <[email protected]>
@akurinnoy
Copy link
Collaborator Author

akurinnoy commented Oct 31, 2025

/retest

dkwon17
dkwon17 reviewed Nov 3, 2025
View reviewed changes
controllers/controller/devworkspacerouting/solvers/common.go
// GetDiscoverableServicesForEndpoints converts the endpoint list into a set of services, each corresponding to a single discoverable
// endpoint from the list. Endpoints with the NoneEndpointExposure are ignored.
func GetDiscoverableServicesForEndpoints(endpoints map[string]controllerv1alpha1.EndpointList, meta DevWorkspaceMetadata) []corev1.Service {
func GetDiscoverableServicesForEndpoints(endpoints map[string]controllerv1alpha1.EndpointList, meta DevWorkspaceMetadata, cl client.Client, log logr.Logger) ([]corev1.Service, error) {
Copy link
Collaborator

@dkwon17 dkwon17 Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akurinnoy is there a reason for implementing duplicate endpiont detection in both the webhook and the GetDiscoverableServicesForEndpoints function?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rohanKanojia rohanKanojia rohanKanojia left review comments

@dkwon17 dkwon17 dkwon17 left review comments

@tolusha tolusha tolusha approved these changes

@ibuziuk ibuziuk Awaiting requested review from ibuziuk ibuziuk is a code owner

Assignees

@tolusha tolusha

@akurinnoy akurinnoy

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Dev Workspace Controller Loop with Identical Endpoint Names on DevWorkspace

4 participants

@akurinnoy @rohanKanojia @tolusha @dkwon17